Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: type-check trait default methods #6645

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

asterite
Copy link
Collaborator

@asterite asterite commented Nov 28, 2024

Description

Problem

Resolves #6519

Summary

This is a breaking change because this code used to compile:

trait Foo {
  fn foo(self) -> Field {
    self.x
  }
}

That is, you could access a member of self in a trait method. Because the trait default method was checked against the actual concrete type, if it had a member x it compiled fine. Now that code is rejected (like in Rust). And... we have code like that in Aztec-Packages.

Additional Context

Documentation

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@asterite asterite changed the title fix: type-check trait default methods feat!: type-check trait default methods Dec 23, 2024
@asterite asterite force-pushed the ab/type-check-trait-default-methods branch 2 times, most recently from 150b2b4 to 52c1332 Compare December 23, 2024 19:15
@asterite asterite force-pushed the ab/type-check-trait-default-methods branch from 52c1332 to 742b8d0 Compare December 23, 2024 19:15
Copy link
Contributor

github-actions bot commented Dec 23, 2024

Peak Memory Sample

Program Peak Memory
keccak256 78.48M
workspace 123.67M
regression_4709 422.91M
ram_blowup_regression 1.58G
private-kernel-tail 201.82M
private-kernel-reset 717.08M
private-kernel-inner 291.89M
parity-root 172.14M

Copy link
Contributor

github-actions bot commented Dec 23, 2024

Compilation Sample

Program Compilation Time
sha256_regression 1.335s
regression_4709 0.803s
ram_blowup_regression 15.374s
private-kernel-tail 1.010s
private-kernel-reset 7.142s
private-kernel-inner 2.173s
parity-root 0.802s
noir-contracts 15.696s

Copy link
Contributor

github-actions bot commented Dec 23, 2024

Execution Sample

Program Execution Time
sha256_regression 0.618s
regression_4709 0.395s
ram_blowup_regression 4.270s
private-kernel-tail 0.681s
private-kernel-reset 1.469s
private-kernel-inner 0.980s
parity-root 0.562s

@asterite asterite marked this pull request as ready for review December 23, 2024 20:28
…-lang/noir into ab/type-check-trait-default-methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid paths in default trait method only error when trait is implemented
1 participant